Manchester | 26-ITP-Jan | Ofonime Edak| Sprint 3 | Dead Code#1232
Manchester | 26-ITP-Jan | Ofonime Edak| Sprint 3 | Dead Code#1232Ofonimeedak wants to merge 8 commits intoCodeYourFuture:mainfrom
Conversation
Ofonimeedak
added
📅 Sprint 3
SlideGauge
added
the
Review in progress
SlideGauge
left a comment
SlideGauge
left a comment
There was a problem hiding this comment.
The description of the task says "One pull request for the 3-dead-code directory."
Could you adjust your pull request according to the task please?
| testName = "Aman"; | ||
|
|
||
| const greetingMessage = sayHello(greeting, testName); | ||
| const greetingMessage = sayHello('hello', 'Aman!'); |
There was a problem hiding this comment.
Will there be a difference between the output with these arguments compared to the initial arguments?
| console.log(greetingStr); | ||
| } | ||
|
|
||
| testName = "Aman"; |
There was a problem hiding this comment.
It is normal to put arguments themselves into variables and, better, constants, instead of pasting raw literals into function arguments. These raw literals are called "magic numbers"/"magic strings" respectively and typically should be avoided.
| const greeting = "hello"; | ||
|
|
||
| function sayHello(greeting, name) { | ||
| const greetingStr = greeting + ", " + name + "!"; |
There was a problem hiding this comment.
Question: after we removed the console.log on the line 10, what is the remaining purpose of this string?
| const capitalisedPets = pets.map((pet) => pet.toUpperCase()); | ||
| const petsStartingWithH = pets.filter((pet) => pet[0] === "h"); | ||
|
|
||
| function logPets(petsArr) { |
There was a problem hiding this comment.
Could you analyse please who's calling this function in the file?
SlideGauge
added
Reviewed
|
There are several things worth fixing, could you do it please? |
2 participants
Self checklist
Changelist
Changes to file exercise-1.js: Identifying redundant code
Changes to file exercise-2.js: Identifying redundant code
Removing all redundant/dead code and testing the function